feat(keymap): add page-sized scroll_diff actions - #358
Merged
Conversation
J/K scroll the diff viewport from either pane but only by a wheel step, while pgdown/pgup and ctrl+d/ctrl+u stay pane-relative — with the tree focused they page the tree, so skimming a file means tabbing into the diff and back. Add scroll_diff_page_down/up and scroll_diff_half_page_down/up, the page-sized siblings of scroll_diff_down/up. Dispatched in both handleTreeAction (inside the early-return switch, ahead of the mdTOC branch, so tree and TOC selection stay put) and handleDiffMovement, so a globally mapped key does not go dead when focus moves to the diff. They ship with no default binding: pgdown/pgup and ctrl+d/ctrl+u are the only keys that page the file tree and the markdown TOC, and repointing them would take that away. Bind them to get lazygit-style paging. Full-page uses pageRows() and so honors --page-overlap; half-page uses the new halfPageRows() and does not, matching the rule the cursor-motion half-page keys already follow. Related to #357
There was a problem hiding this comment.
🟢 Approval recommended
The change is well-scoped, test-covered, and preserves existing defaults; only a minor comment-clarity nit was identified.
Pull request overview
Adds new keymap actions to page/half-page scroll the diff viewport even when focus is in the file tree/TOC, enabling “always scroll the diff” paging without changing any default bindings.
Changes:
- Introduces four new actions (
scroll_diff_{page,half_page}_{down,up}) and registers them as valid keymap actions with help descriptions. - Handles the new actions in both diff-focused and tree-focused action dispatch paths, reusing existing page sizing (
pageRows()) and adding a sharedhalfPageRows()helper. - Adds targeted tests for focus behavior, overlap handling, reverse scrolling, and TOC selection stability; updates user docs and plugin reference docs.
File summaries
| File | Description |
|---|---|
app/keymap/keymap.go |
Adds new action constants, marks them valid, and provides help descriptions (still omitted from help/dump until bound). |
app/keymap/keymap_test.go |
Verifies validity, no default bindings, and help-section behavior once bound. |
app/ui/diffnav.go |
Implements page/half-page diff viewport scrolling actions in both tree and diff dispatch; adds halfPageRows() helper. |
app/ui/diffnav_test.go |
Adds integration-style tests that load a keybindings file and assert viewport/cursor/focus/TOC invariants. |
README.md |
Documents how to bind the new actions and the behavioral consequences of remapping paging keys. |
site/docs.html |
Mirrors README documentation updates for the website docs. |
.claude-plugin/skills/revdiff/references/config.md |
Updates action list and adds guidance for binding the new actions. |
plugins/codex/skills/revdiff/references/config.md |
Same reference-doc updates for the Codex plugin copy. |
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+142
to
+143
| // rows skipped unseen - when worthRollingBack accepts it whole. the scroll_diff_page_* | ||
| // actions reuse this distance on a pure viewport scroll, where it is exact. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
J/Kscroll the diff viewport from either pane, but only by a wheel step.pgdown/pgupandctrl+d/ctrl+ustay pane-relative, so with the tree focused they page the tree and skimming a file means tabbing into the diff and back.Adds four page-sized siblings of
scroll_diff_down/scroll_diff_up:scroll_diff_page_downscroll_diff_page_upscroll_diff_half_page_downscroll_diff_half_page_upDispatched in
handleTreeAction, inside the early-return switch ahead of the mdTOC branch so tree and TOC selection stay put, and inhandleDiffMovementso a globally mapped key does not go dead when focus moves to the diff.No default binding, by design.
pgdown/pgup/ctrl+d/ctrl+uare the only keys that page the file tree, the markdown TOC and the help/info overlays, so repointing them would take that away. Bind them yourself for lazygit-style paging:Full page uses
pageRows()and honors--page-overlap. Half page uses a newhalfPageRows()and does not, matching the rule the cursor-motion half-page keys already follow from #313. That helper also replaces the two inline copies ofmax(1, Height/2)in the existing half-page motions.Tests drive a real keybindings file through
keymap.Loadand thenUpdate: both focus modes, cursor pinned at the new viewport top, focus and tree selection unchanged, up reversing down, overlap subtracted from full page and ignored by half, TOC cursor unmoved. Docs updated in README,site/docs.htmland both pluginreferences/config.mdcopies, with the remap example and its three consequences.Most of the
app/keymap/keymap.godiff is gofmt realigning the const block: the longest new identifier moved the alignment column.Fix #357